Skip to content

[Preview] Adding support for Standard Metrics#1002

Merged
hectorhdzg merged 9 commits intopreviewfrom
hectorhdzg/standradmetrics
Aug 17, 2022
Merged

[Preview] Adding support for Standard Metrics#1002
hectorhdzg merged 9 commits intopreviewfrom
hectorhdzg/standradmetrics

Conversation

@hectorhdzg
Copy link
Copy Markdown
Member

Added duration metrics for server/client in internal MetricsInstrumentation
Exceptions and traces standard metrics continue to work as before
Added SpanProcessor to add extra properties in spans

private _httpRequestDone(metric: IHttpStandardMetric) {
// Done could be called multiple times, only process metric once
if (!metric.isProcessed) {
metric.isProcessed = true;
Copy link
Copy Markdown

@lzchen lzchen Aug 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious as to why and how _httpRequestDone is called multiple times for the same request?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be called multiple times for specific cases, like request timeout, abort, etc.

if (this._metricHandler?.isStandardMetricsEnabled) {
let exceptionDimensions: IMetricExceptionDimensions = {
cloudRoleInstance: envelope.tags[KnownContextTagKeys.AiCloudRoleInstance],
cloudRoleName: envelope.tags[KnownContextTagKeys.AiCloudRole]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another field client/type might be needed. Curious did you look into what this dimension represents?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not aware of the existence of that dimension, currently only enabling what we had before


private static _instance: HttpMetricsInstrumentation;
private _nodeVersion: string;
public totalRequestCount: number = 0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to keep track of these totals? Are you planning to replace performance counters with this instrumentation?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfomance counters are currently calculated using these, is still not very clear how perf counters would be handled in OpenTelemetry so having this functionality in the meantime

@hectorhdzg
Copy link
Copy Markdown
Member Author

Exporter related changes: Azure/azure-sdk-for-js#22886

@hectorhdzg hectorhdzg merged commit 2b9e855 into preview Aug 17, 2022
@hectorhdzg hectorhdzg deleted the hectorhdzg/standradmetrics branch November 9, 2022 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants